Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FF122 Clipboard API #31137

Merged
merged 32 commits into from
Jan 9, 2024
Merged

FF122 Clipboard API #31137

merged 32 commits into from
Jan 9, 2024

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Dec 19, 2023

FF122 adds support for the async clipboard API. This PR updates the docs to spec states.

The first commit can largely be ignored as a "subedit for current templates/patterns".

  • Fixes layout issues such as odd line breaks.
  • Adds securecontext header and inline in the places we have now agreed they need to go
  • Obeys current patterns "The XXX interface of the YYY API ...", "The XXX property of the YYY interface" etc.
  • Deletes all the comment about what a new API this is.

The remainder of this was to remove Permissions API information, which was partially done in #12066, and to add some exceptions information. Also fix up an example.

The permissions stuff is a little hard to resolve because they have been sloppily removed from spec - so we know that the permissions aren't used, but it's hard to work out what actually is done. They are still used in Chrome too. I have "resolved" this primarily by removing mention of permissions but keeping a generic exception for NotAllowed. I've also noted that browsers might add additional restrictions on the top level API doc (which captures both the permissions and also that FF asks for a prompt on cross-origin content read.

Could also update the top level Clipboard API to have additional examples. But possibly as a follow on.

Related docs project can be tracked in #31104

I probably should add permission info to BCD about permissions too.

@hamishwillee hamishwillee requested a review from a team as a code owner December 19, 2023 04:53
@hamishwillee hamishwillee requested review from sideshowbarker and removed request for a team December 19, 2023 04:53
@github-actions github-actions bot added the Content:WebAPI Web API docs label Dec 19, 2023
Copy link
Contributor

github-actions bot commented Dec 19, 2023

Preview URLs (21 pages)
Flaws (1)

Note! 20 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/Navigator
Title: Navigator
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/Navigator/standalone does not exist
External URLs (4)

URL: /en-US/docs/Web/API/Permissions_API
Title: Permissions API


URL: /en-US/docs/Web/API/Clipboard_API
Title: Clipboard API

(comment last updated: 2024-01-09 04:22:00)

@@ -148,12 +148,7 @@
"interfaces": ["Clipboard", "ClipboardEvent", "ClipboardItem"],
"methods": [],
"properties": ["Navigator.clipboard"],
"events": [
"Window: clipboardchange",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Window: clipboardchange is not implemented anywhere.

@hamishwillee hamishwillee requested a review from a team as a code owner December 28, 2023 23:46
@github-actions github-actions bot added the Content:Security Security docs label Dec 28, 2023
@hamishwillee
Copy link
Collaborator Author

This is ready for review. There is more I could do, and a little more I might need to do, but I think it is good for now.

@wbamberg
Copy link
Collaborator

wbamberg commented Jan 3, 2024

The stuff on permissions seems to be a complicated mess, so we should take care to explain it as well as we can.

As I understand it:

  • clipboard read and clipboard write both require transient user activation in the spec and all browser implementations
  • Chromium also defines clipboard-read and clipboard-write permissions, but these are not supported (and will never be supported) by Firefox or Safari
  • clipboard-read is no longer in the spec (so Chromium's support is non-standard) but clipboard-write is still in the spec (so is technically standard)

Does that cover it? Do you think we need to say something like this?

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Jan 4, 2024

@wbamberg Your summary in #31137 (comment) is almost right.

  • clipboard read and clipboard write both require transient user activation in the spec and all browser implementations
  • Chromium also defines clipboard-read and clipboard-write permissions, but these are not supported (and will never be supported) by Firefox or Safari

This is correct

  • clipboard-read is no longer in the spec (so Chromium's support is non-standard) but clipboard-write is still in the spec (so is technically standard)

The distinction is that things are even more messy than you think.

  • The definitions for permission API permissions were documented in the Permissions API spec but have since been removed. So clipboard spec is referencing permissions that do not exist in-spec.
  • The clipboard-read permission was "agreed removed" but is still "partially" referenced in clipboard API - i.e. with statements like "check read permission"
  • The clipboard-write permission is noted as removed in the spec, but is referenced, and in the issue you point to it looks a bit like the spec authors were perhaps premature to remove it from Permissions API

Upshot, the spec is so confused now it is IMO unimplementable, and so what the spec requires is now a matter of opinion. Given the notes in the spec and comments in the PR I made the decision that we should treat permissions as removed from the spec.
Partially this is because I agree with the decision to remove those permissions - the experience on chrome is (subjectively) worse, to benefit.

So what I did say in concepts section was treat permissions as a thing that a browser can choose to do:

image

I've tried to make this even more clear by adding a new note at the end of the concepts section to capture "the facts". Does this do what you need?

image

@hamishwillee
Copy link
Collaborator Author

PS, and I removed the clipboard API from the list of APIs that have permissions in Permission API. I see this as reasonable because the Permissions API spec does not mention the permissions (whether that was premature or not, it is what the spec says).

@hamishwillee hamishwillee force-pushed the ff122clipboard_api branch 2 times, most recently from 47444d1 to e0d3b01 Compare January 9, 2024 00:32
@hamishwillee
Copy link
Collaborator Author

@wbamberg I've accepted all the nit fixes. Now trying to work out what is going on with the non-rendering image. Frankly I am stumped, and not sure what to do next.

  1. I can see the image is present in the folder and it displays locally.
  2. On upload it is "not found". There is no error in the build.
  3. If I use a different image it does not work. I thought maybe there was something wrong with the image I used, so I tried adding a few more new images and copies of the JPG that works (butterfly). Only the original butterfly renders.
  4. I also tried pushing this as a new PR - [TEST ONLY - DONT MERGE] ff122clipimages #31592 . I thought perhaps original PR was not rendering. Again, it only renders the original butterfly that was uploaded - not even a duplicate butterfly with a new name is displayed.

Any suggestions? I think this is not a bug in my code. I'd like to merge and see if it is something to do with the PR reviewer.

@wbamberg
Copy link
Collaborator

wbamberg commented Jan 9, 2024

I don't think this is a bug in your code either. My suggestion:

  • remove the PNG example
  • file a Yari issue for the problem. We can use your test PR as a demonstration of the problem.
  • merge this PR. I don't think we should on this problem.


- Even though the butterfly image is a JPG file, when read from the clipboard it is a PNG.
- If prompted, you will need to grant permission in order to paste the image.
- This may not work on chromium browsers as the sample frame is not granted the [Permissions-Policy](/en-US/docs/Web/HTTP/Headers/Permissions-Policy) `clipboard-read` and `clipboard-write` permissions ([required by Chromium browsers](/en-US/docs/Web/API/Clipboard_API#security_considerations)).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wbamberg As discussed, I couldn't get the second image to display - created an issue in mdn/yari#10287

This also logs an error on Chromium due to the permissions policy not being granted to the live sample frame. I tried a LiveSampleLink because I figured that might work, but on PR preview that page gives a "Not found".

Anyway, think this is good for a re-review.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks great, thank you for this @hamishwillee !

@wbamberg wbamberg merged commit 7087ffd into mdn:main Jan 9, 2024
8 checks passed
estelle pushed a commit to estelle/content that referenced this pull request Jan 9, 2024
* FF122 Clipboard API - basic layout, style subedit, secure_context

* copy event clarification

* Remove Window: clipboardchange event from GroupData.json - not implemented

* Add Element cut/copy/paste events to overview

* Security considerations

* Events are 'of the Clipboard API'

* Clipboard.write() requires transient activation

* Clipboard.read() - update example and remove permissions

* Clipboard.read*/write* - remove permission mentions

* Clipboard interface - remove permissions details

* Clipboard API - make concepts a little less abstract

* Navigator.clipboard - remove the text on requiring permissions

* Minor tweaks

* Permissions API- remove clipboard permissions

* Add note about Chromium behaviour

* read() example handles the permissions handled/or not handled

* Merge specific suggestions from code review

Co-authored-by: wbamberg <[email protected]>

* Fix example as per feedback

* read - add example for more reading of data

* Remove guide on interacting with web extensions

* Security requirements

* Add security considerations to the other methods

* Fix typos. Update readText

* Make examples async

* Remove that write one at a time comment

* Update files/en-us/web/api/clipboard_api/index.md

* typo

* Apply suggestions from code review

Co-authored-by: wbamberg <[email protected]>

* Update files/en-us/web/api/clipboard/read/index.md

* Test image source has effect on rendering

* Remove clock (not rendering). Try sample link for chromium

* Remove the live sample link

---------

Co-authored-by: wbamberg <[email protected]>
@hamishwillee hamishwillee deleted the ff122clipboard_api branch January 9, 2024 07:35
@hamishwillee
Copy link
Collaborator Author

It only looks this way because of your help and patience. Thank you.

dipikabh pushed a commit to dipikabh/content that referenced this pull request Jan 17, 2024
* FF122 Clipboard API - basic layout, style subedit, secure_context

* copy event clarification

* Remove Window: clipboardchange event from GroupData.json - not implemented

* Add Element cut/copy/paste events to overview

* Security considerations

* Events are 'of the Clipboard API'

* Clipboard.write() requires transient activation

* Clipboard.read() - update example and remove permissions

* Clipboard.read*/write* - remove permission mentions

* Clipboard interface - remove permissions details

* Clipboard API - make concepts a little less abstract

* Navigator.clipboard - remove the text on requiring permissions

* Minor tweaks

* Permissions API- remove clipboard permissions

* Add note about Chromium behaviour

* read() example handles the permissions handled/or not handled

* Merge specific suggestions from code review

Co-authored-by: wbamberg <[email protected]>

* Fix example as per feedback

* read - add example for more reading of data

* Remove guide on interacting with web extensions

* Security requirements

* Add security considerations to the other methods

* Fix typos. Update readText

* Make examples async

* Remove that write one at a time comment

* Update files/en-us/web/api/clipboard_api/index.md

* typo

* Apply suggestions from code review

Co-authored-by: wbamberg <[email protected]>

* Update files/en-us/web/api/clipboard/read/index.md

* Test image source has effect on rendering

* Remove clock (not rendering). Try sample link for chromium

* Remove the live sample link

---------

Co-authored-by: wbamberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Security Security docs Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants